chore: add jgit spans inside resourceClosure by dvj1988 · Pull Request #39737 · appsmithorg/appsmith

dvj1988 · 2025-03-17T03:39:32Z

Description

Add jgit spans inside resourceClosure to verify if it is same as the parent span.

Fixes #Issue Number
or
Fixes Issue URL

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags="@tag.Git"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13892637993
Commit: 923c9ab
Cypress dashboard.
Tags: @tag.Git
Spec:

Mon, 17 Mar 2025 06:50:49 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

Yes
No

Summary by CodeRabbit

New Features
- Enhanced monitoring and observability for Git operations now provide improved performance insights and reliability.
- Expanded tracking across version control activities delivers more precise diagnostics and smoother troubleshooting, resulting in a more stable and efficient Git experience for users.

coderabbitai · 2025-03-17T03:39:40Z

Walkthrough

The pull request introduces enhanced observability across various Git handlers and executors. Constructors in both standard and CE implementations now require an ObservationHelper instance, and new span tracking calls have been integrated into multiple Git operations. Additionally, new constants for JGit operations have been added to improve traceability in Git activities.

Changes

File(s)	Change Summary
`app/server/appsmith-git/.../handler/FSGitHandlerImpl.java`, `app/server/appsmith-git/.../service/GitExecutorImpl.java`	Updated constructors to include an additional `ObservationHelper` parameter and adjusted superclass calls accordingly.
`app/server/appsmith-git/.../handler/ce/FSGitHandlerCEImpl.java`, `app/server/appsmith-git/.../service/ce/GitExecutorCEImpl.java`	Introduced a new `observationHelper` field and inserted span creation/termination calls in key Git operation methods for enhanced monitoring.
`app/server/appsmith-interfaces/.../constants/ce/GitSpanCE.java`	Added multiple new constants for various JGit operations to standardize span identifiers.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Git Client
    participant Handler as Git Handler (CE)
    participant Obs as ObservationHelper
    Client->>Handler: Trigger Git Operation (e.g., commitArtifact)
    Handler->>Obs: createSpan("Operation Span")
    Handler->>Handler: Execute Git Operation
    Handler->>Obs: span.end()
    Handler->>Client: Return Result

Possibly related PRs

chore: git changes for new contract #39376: Involved similar modifications integrating ObservationHelper into the FSGitHandlerImpl constructor, focusing on dependency management and constructor updates.

Suggested labels

JS Evaluation, Task, JS Objects, Javascript Product, Query & JS Pod, Git Product

Suggested reviewers

sharat87
nidhi-nair
ashit-rath

Poem

In realms of Git where code takes flight,
Observations guide our sight.
Spans now track each commit and push,
Constructors updated in a hush.
With constants clear as morning dew,
Our code sings bright and true 🚀.
Cheers to progress and a job well done!

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)

We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
- To enable this feature, set early_access to true under in the settings.

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f420384 and 923c9ab.

📒 Files selected for processing (5)

app/server/appsmith-git/src/main/java/com/appsmith/git/handler/FSGitHandlerImpl.java (2 hunks)
app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java (39 hunks)
app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java (2 hunks)
app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java (34 hunks)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (3)

GitHub Check: perform-test / server-build / server-unit-tests
GitHub Check: server-spotless / spotless-check
GitHub Check: server-unit-tests / server-unit-tests

🔇 Additional comments (35)

app/server/appsmith-git/src/main/java/com/appsmith/git/handler/FSGitHandlerImpl.java (2)

5-5: New import for ObservationHelper
The added import aligns with the new observability requirement.

19-23: Overloaded constructor with ObservationHelper
The additional parameter is passed correctly to the superclass, ensuring consistent initialization.

app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java (13)

12-12: Class-level additions for ObservationHelper
New imports and field injection look good and set up consistent span creation.

Also applies to: 23-23, 96-96

126-126: Spans for commitArtifact (add & commit)
These spans neatly capture JGit add and commit operations.

Also applies to: 139-139, 142-142, 151-151

238-238: Span usage in pushApplication
Capturing the push operation’s timing is implemented cleanly.

Also applies to: 269-269

298-298: Span usage in cloneRemoteIntoArtifactRepo
The clone operation’s duration is now traceable with no apparent issues.

Also applies to: 319-319

337-338: Span usage in createAndCheckoutToBranch
Branch creation instrumentation matches the established pattern.

Also applies to: 359-361

373-374: Span usage in deleteBranch
Branch deletion steps are properly measured.

Also applies to: 440-440

405-406: Span usage in checkoutToBranch
No concerns; the checkout flow is now observable.

Also applies to: 424-424

447-447: Span usage in pullApplication
Pull metrics are captured consistently.

Also applies to: 473-473

503-503: Span usage in pullArtifactWithoutCheckout
Aligns seamlessly with the other pull method’s approach.

Also applies to: 526-526

712-712: Span usage in getStatus
Status checks now have clear timing instrumentation.

Also applies to: 765-765

958-958: Span usage in mergeBranch
Merges are traced accurately, with no functional concerns.

Also applies to: 982-982

1015-1015: Span usage in fetchRemote (first overload)
Timing is tracked effectively for the remote fetch.

Also applies to: 1036-1036

1057-1057: Span usage in fetchRemote (second overload)
Consistent approach to measuring multi-branch or tag fetch operations.

Also applies to: 1062-1062, 1067-1067, 1068-1068, 1091-1091, 1099-1099, 1100-1100

app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java (2)

5-5: Import for ObservationHelper
No issues; this aligns with the extended observability features.

18-22: Constructor now accepts ObservationHelper
Forwarding the parameter to the superclass properly enables trace creation.

app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java (17)

15-15: Imports and field for ObservationHelper
Sets a solid foundation for creating spans in subsequent methods.

Also applies to: 26-26, 105-105

134-134: Spans for commitArtifact
JGit add and commit instrumentation is done consistently.

Also applies to: 147-147, 150-150, 159-159, 160-160

306-306: Span usage in cloneRemoteIntoArtifactRepo
Capturing clone duration adds helpful observability.

Also applies to: 327-327

345-346: Span usage in createAndCheckoutToBranch
Branch creation instrumentation is consistent and clear.

Also applies to: 359-360, 362-362

429-430: Span usage in deleteBranch
Branch deletion is now properly traced.

Also applies to: 440-440

462-463: Span usage in checkoutToBranch
No concerns; this approach follows the existing pattern.

Also applies to: 483-483

503-503: Span usage in pullArtifactWithoutCheckout
Mirrors the pattern from other pull operations effectively.

Also applies to: 526-526

574-574: Span usage in pullApplication
Pull logic observability is consistent and well-placed.

Also applies to: 600-600

712-712: Span usage in getStatus
Status instrumentation is introduced without disrupting existing logic.

Also applies to: 765-765

958-958: Span usage in mergeBranch
Merges gain clear trace metrics. Implementation looks good.

Also applies to: 982-982

1015-1015: Span usage in fetchRemote
Fetch times are now clearly captured.

Also applies to: 1036-1036

1057-1057: Enhanced fetchRemote method
Instrumentation remains aligned with the existing span creation pattern.

Also applies to: 1062-1062, 1067-1067, 1068-1068, 1091-1091, 1099-1099, 1100-1100

1201-1201: Span usage in checkoutRemoteBranch
Provides trace coverage for remote checkouts as well.

Also applies to: 1220-1221

1252-1252: Span usage in resetToLastCommit
Effectively tracks the reset and clean actions.

Also applies to: 1255-1255, 1257-1257, 1259-1259

1284-1284: Span usage in resetHard
Covers forced reset operations consistently with the rest.

Also applies to: 1289-1289

1308-1308: Span usage in rebaseBranch
Properly handles success and error flows within a single span context.

Also applies to: 1313-1313, 1324-1324

1345-1345: Span usage in getBranchTrackingStatus
Now measuring the branch tracking retrieval.

Also applies to: 1349-1350

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java (1)

9-38: Well-structured addition of JGit span constants

The addition of JGit span constants provides a comprehensive parallel to the existing FS span constants, enabling proper tracing for JGit operations. The naming convention is consistent and follows the established pattern.

✨ Finishing Touches

📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2025-03-17T04:06:12Z

Failed server tests

com.appsmith.server.git.ServerSchemaMigrationEnforcerTest#saveGitRepo_ImportAndThenExport_diffOccurs

## Description Add jgit spans inside resourceClosure to verify if it is same as the parent span. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results  > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/13892637993> > Commit: 923c9ab > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13892637993&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Git` > Spec: > <hr>Mon, 17 Mar 2025 06:50:49 UTC  ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No  ## Summary by CodeRabbit - **New Features** - Enhanced monitoring and observability for Git operations now provide improved performance insights and reliability. - Expanded tracking across version control activities delivers more precise diagnostics and smoother troubleshooting, resulting in a more stable and efficient Git experience for users.

chore: add jgit spans

923c9ab

dvj1988 requested a review from rajatagrawal March 17, 2025 03:39

dvj1988 added the ok-to-test Required label for CI label Mar 17, 2025

github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Mar 17, 2025

dvj1988 marked this pull request as ready for review March 17, 2025 03:40

dvj1988 requested a review from a team as a code owner March 17, 2025 03:40

dvj1988 added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Mar 17, 2025

rajatagrawal approved these changes Mar 17, 2025

View reviewed changes

dvj1988 merged commit 2420f90 into release Mar 17, 2025
110 of 121 checks passed

dvj1988 deleted the chore/add-spans-git-monos branch March 17, 2025 08:47

This was referenced Mar 17, 2025

chore: remove analytics execution from the critical path #39757

Merged

chore: spans added in ast service for RTS calls #39823

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add jgit spans inside resourceClosure#39737

chore: add jgit spans inside resourceClosure#39737
dvj1988 merged 1 commit intoreleasefrom
chore/add-spans-git-monos

dvj1988 commented Mar 17, 2025 •

edited by github-actions bot

Loading

Uh oh!

coderabbitai bot commented Mar 17, 2025 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

github-actions bot commented Mar 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dvj1988 commented Mar 17, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Automation

🔍 Cypress test results

Communication

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

github-actions bot commented Mar 17, 2025

Failed server tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dvj1988 commented Mar 17, 2025 •

edited by github-actions bot

Loading

coderabbitai bot commented Mar 17, 2025 •

edited

Loading